feat: add ability to revoke login sessions by SessionID #3450
Conversation
| // has to re-authenticate at the Ory OAuth2 Provider. This endpoint does not invalidate any tokens and | ||
| // does not work with OpenID Connect Front- or Back-channel logout. | ||
| // If you send the subject in a query param, all authentication sessions that belong to that subject are revoked. | ||
| // No OpennID Connect Front- or Back-channel logout is performed in this case. |
There was a problem hiding this comment.
Looks like part of this can be fixed as well (back-channel), maybe outside of this change.
consent/strategy_default.go
Outdated
| return nil | ||
| } | ||
|
|
||
| if err := s.executeBackChannelLogout(r.Context(), r, loginSession.Subject, sid); err != nil { |
There was a problem hiding this comment.
Everything from here until the end of the function is the same as in completeLogout function. Is this the correct direction to take here?
There was a problem hiding this comment.
That looks good to me! Since this is called via an API, we won't have front-channel log-out (because no browser is involved). I think this is fine, but should probably be clarified in the API / function doc. You may opt to create a new function with the flow as to avoid duplication, but it's not required since it's only 2 functional statements
sgal
changed the title
consent/strategy_default.go
Outdated
| s.r.AuditLogger(). | ||
| WithRequest(r). | ||
| WithField("subject", loginSession.Subject). | ||
| Info("User logout completed!") |
There was a problem hiding this comment.
Clarify that this is the headless flow
consent/strategy_default.go
Outdated
| return nil | ||
| } | ||
|
|
||
| if err := s.executeBackChannelLogout(r.Context(), r, loginSession.Subject, sid); err != nil { |
There was a problem hiding this comment.
That looks good to me! Since this is called via an API, we won't have front-channel log-out (because no browser is involved). I think this is fine, but should probably be clarified in the API / function doc. You may opt to create a new function with the flow as to avoid duplication, but it's not required since it's only 2 functional statements
Codecov Report
@@ Coverage Diff @@
## master #3450 +/- ##
==========================================
- Coverage 76.86% 76.85% -0.01%
==========================================
Files 123 123
Lines 9108 9131 +23
==========================================
+ Hits 7001 7018 +17
- Misses 1663 1666 +3
- Partials 444 447 +3
... and 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
consent/strategy_logout_test.go
Outdated
| @@ -490,4 +509,24 @@ func TestLogoutFlows(t *testing.T) { | |||
|
|
|||
| wg.Wait() | |||
| }) | |||
|
|
|||
| t.Run("case=should execute backchannel logout in headless flow with sid", func(t *testing.T) { | |||
| sid := make(chan string, 2) | |||
There was a problem hiding this comment.
Using a buffered channel here to make sure that we have both values in it and do not block the goroutine that tries to read it.
|
@hperl Fixed the comments, please have a look. |
|
Hey @aeneasr, when are you planning to release this change? |
API `revokeOAuth2LoginSessions` can now revoke a single session by a SessionID (`sid` claim in the id_token) and execute an OpenID Connect Back-channel logout. Closes ory#3448
Added support to
revokeOAuth2LoginSessionsAPI to revoke a single session by a SessionID (sidclaim in the id_token) and execute an OpenID Connect Back-channel logout.Related issue(s)
#3448
Checklist
introduces a new feature.
contributing code guidelines.
vulnerability. If this pull request addresses a security vulnerability, I
confirm that I got the approval (please contact
security@ory.sh) from the maintainers to push
the changes.
works.
Further Comments